codespell: add config and action to codespell the code to avoid known typos by yarikoptic · Pull Request #26194 · go-gitea/gitea

yarikoptic · 2023-07-28T02:26:23Z

With this PR merged workflow would fail if code introduces new typos.

One commit tunes up .gitignore.

wxiaoguang · 2023-07-28T05:08:29Z

"fomantic/build/semantic.js" shouldn't be touched.

wxiaoguang · 2023-07-28T05:09:03Z

Are you sure this change is right?

please advise! I do not know gitea (yet). But I know that startd is not a word ;-)

I don't understand it (sunos) either, but I think it can be googled:

https://www.google.com/search?q=sunos+framework+%22startd%22

d stands for daemon in this case.

Yup, it seems a predefined name in sunos, but I have no experience with sunos.

ok, I will leave this startd alone

wxiaoguang · 2023-07-28T05:10:02Z

Is it really a typo? Or it is so, or would it case breaking? @KN4CK3R

That's a typo and would be a breaking change. Needs to fixed with a migration.

Please advise on how to proceed -- e.g. I could exclude this from being fixed in this PR and you then do in a separate PR if decide to proceed fixing it (note that there were other similar ones like in committer IIRC which I had to exclude to not break API)

Do you think it is a must to rename it and use a migration to update legacy data?

I haven't read the code carefully, I guess it could be one of these cases:

No need to rename it, just leave a comment that "it's a typo, but it doesn't affect end users".

The typo affects end users, and it must be renamed and get a migration to update legacy data.

The typo doesn't affect end user, but it's better to rename it with a migration.

Which case do you think it is?

The migration is as simple as UPDATE package_property SET name = 'conda.metadata' WHERE name = 'conda.metdata'. I will create a PR soonish.

wxiaoguang

I am afraid there are too many problems by changing typos blindly.

Every change should be manually checked.

wxiaoguang · 2023-07-28T05:15:01Z

!/custom/conf should be ignored because it contains local "app.ini"

without this line unfortunately git does not commit changes for the specific file listed below, I guess because the above /custom/* has a *... that is just my guess. May be worth then ignoring specifically /custom/conf/app.ini and not just /custom/*?

without this line unfortunately git does not commit changes for the specific file listed below

Which file?

TBH I don't see a reason why it needs to add "!/custom/conf" into the list.

hm, I have tried to reproduce my (well -- datalad) problem committing those changes and failed

❯ git clone https://github.com/go-gitea/gitea/ Cloning into 'gitea'... remote: Enumerating objects: 228164, done. remote: Counting objects: 100% (1393/1393), done. remote: Compressing objects: 100% (1057/1057), done. ^Cceiving objects: 9% (21905/228164), 9.76 MiB | 3.88 MiB/s ❯ git clone https://github.com/go-gitea/gitea/ ❯ rm -rf gitea ❯ git clone --depth 1 https://github.com/go-gitea/gitea/ Cloning into 'gitea'... remote: Enumerating objects: 6266, done. remote: Counting objects: 100% (6266/6266), done. remote: Compressing objects: 100% (5456/5456), done. remote: Total 6266 (delta 602), reused 2877 (delta 380), pack-reused 0 Receiving objects: 100% (6266/6266), 10.02 MiB | 5.14 MiB/s, done. Resolving deltas: 100% (602/602), done. ❯ cd gitea BSDmakefile Makefile custom/ package-lock.json snap/ CHANGELOG.md README.md docker/ package.json templates/ CODE_OF_CONDUCT.md README_ZH.md docs/ playwright.config.js tests/ CONTRIBUTING.md SECURITY.md go.mod poetry.lock vitest.config.js DCO assets/ go.sum poetry.toml web_src/ Dockerfile build/ main.go public/ webpack.config.js Dockerfile.rootless build.go models/ pyproject.toml LICENSE cmd/ modules/ routers/ MAINTAINERS contrib/ options/ services/ ❯ echo change >> ./custom/conf/app.example.ini changes on filesystem: custom/conf/app.example.ini | 1 + ❯ git commit -m 'Can we commit?' ./custom/conf/app.example.ini [main ee3c3bd] Can we commit? 1 file changed, 1 insertion(+) ❯ git reset HEAD^ Unstaged changes after reset: M custom/conf/app.example.ini changes on filesystem: custom/conf/app.example.ini | 1 + ❯ git commit -m 'Can we commit?' -a [main f91638a] Can we commit? 1 file changed, 1 insertion(+)

so I am not sure why exactly git was refusing to commit for me yesterday any longer.

I also realized that I ended up skipping fixing that ALLWAYS (btw - was it intended to be that instead of "always"?) : https://github.com/go-gitea/gitea/blob/main/custom/conf/app.example.ini#L510

the point is - I will skip this commit

Hm, there are many wrong changes.

yarikoptic · 2023-07-28T13:59:44Z

@lunny please point the specific ones and we will white list them -- I have done that already for a good number and from the review above - only few questionable/problematic are mentioned.

KN4CK3R · 2023-07-28T07:52:22Z

KN4CK3R · 2023-07-28T14:36:54Z

d stands for daemon in this case.

KN4CK3R · 2023-07-28T14:39:53Z

The migration is as simple as UPDATE package_property SET name = 'conda.metadata' WHERE name = 'conda.metdata'. I will create a PR soonish.

lunny · 2023-07-28T15:27:26Z

@lunny please point the specific ones and we will white list them -- I have done that already for a good number and from the review above - only few questionable/problematic are mentioned.

Yes, Have to take look one by one. Because some of them cannot be resolved by simply correcting the spelling.

KN4CK3R · 2023-07-28T15:44:46Z

Is it possible to configure exceptions? Otherwise this job will always fail because it detects false positives.

yes, there is a configuration added to pyproject.toml where I already configured many exceptions: https://github.com/go-gitea/gitea/pull/26194/files#diff-50c86b7ed8ac2cf95bd48334961bf0530cdc77b5a56f852c5c61b89d735fd711R16 .

…N4CK3R

…N4CK3R)

=== Do not change lines below === { "chain": [], "cmd": "codespell -w", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^

yarikoptic · 2023-07-28T22:12:48Z

I have force-pushed rewrite with

this overall diff for the new state from prior one

diff --git a/.gitignore b/.gitignore
index 394af4fad..6b699e087 100644
--- a/.gitignore
+++ b/.gitignore
@@ -53,7 +53,6 @@ cpu.out
 /bin
 /dist
 /custom/*
-!/custom/conf
 !/custom/conf/app.example.ini
 /data
 /indexers
diff --git a/contrib/init/sunos/gitea.xml b/contrib/init/sunos/gitea.xml
index 3e128d5b9..4b8cc3a38 100644
--- a/contrib/init/sunos/gitea.xml
+++ b/contrib/init/sunos/gitea.xml
@@ -31,7 +31,7 @@
 		<exec_method type="method" name="stop"	exec=":kill" timeout_seconds="60"/>
 
 		<property_group name="application" type="application"></property_group>
-		<property_group name="started" type="framework">
+		<property_group name="startd" type="framework">
 		  <propval name="duration" type="astring" value="child"/>
 		  <propval name="ignore_error" type="astring" value="core,signal"/>
 		</property_group>
diff --git a/modules/packages/conda/metadata.go b/modules/packages/conda/metadata.go
index 5eb72b8e3..02dbf313b 100644
--- a/modules/packages/conda/metadata.go
+++ b/modules/packages/conda/metadata.go
@@ -27,7 +27,7 @@ const (
 	PropertyName     = "conda.name"
 	PropertyChannel  = "conda.channel"
 	PropertySubdir   = "conda.subdir"
-	PropertyMetadata = "conda.metadata"
+	PropertyMetadata = "conda.metdata"
 )
 
 // Package represents a Conda package
diff --git a/modules/packages/rpm/metadata.go b/modules/packages/rpm/metadata.go
index f019a8dde..607ea42ea 100644
--- a/modules/packages/rpm/metadata.go
+++ b/modules/packages/rpm/metadata.go
@@ -15,7 +15,7 @@ import (
 )
 
 const (
-	PropertyMetadata = "rpm.metadata"
+	PropertyMetadata = "rpm.metdata"
 
 	SettingKeyPrivate = "rpm.key.private"
 	SettingKeyPublic  = "rpm.key.public"
diff --git a/pyproject.toml b/pyproject.toml
index 17f59687b..e4b973a14 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -18,10 +18,10 @@ ignore="H005,H006,H008,H013,H016,H020,H021,H030,H031"
 skip = '.git,*.pdf,*.svg,package-lock.json,go.mod,locale,license,*.git,objects,*.fr-fr.*,*.de-de.*,*.css,go.sum,*.key,gitignore,pyproject.toml,diff_test.go,go-licenses.json'
 # precise hits for CamelCased words,various other curious cases which require regex to ignore
 # entire line or some portion of it
-ignore-regex = '(\b(mx claus|commitT|ReadBy|#afile|respOne|commitI|[cC]rossReference)\b|shouldbe\.|women’s.*womens|"emoji":.*|,bu,|assert\.Equal.*"fo\b|github\.com/unknwon|Copyright 2014 Unknwon|allowed\.noone|[hH]eadErr|atLeast|{"\\U.*)'
+ignore-regex = '(\b(mx claus|commitT|ReadBy|#afile|respOne|commitI|[cC]rossReference)\b|shouldbe\.|women’s.*womens|"emoji":.*|,bu,|assert\.Equal.*"fo\b|github\.com/unknwon|Copyright 2014 Unknwon|allowed\.noone|[hH]eadErr|atLeast|{"\\U.*|\.metdata)'
 #|.*(Maskenpflicht|Geimpft),.*)'
 # te - TreeEntry variable
 # commiter - wrong spelling but seems used in API
 # ALLWAYS - is a config var
 # infact - other variable(s)
-ignore-words-list = 'crate,te,commiter,befores,allways,infact'
+ignore-words-list = 'crate,te,commiter,befores,allways,infact,startd'

yarikoptic · 2023-07-28T22:13:25Z

+skip = '.git,*.pdf,*.svg,package-lock.json,go.mod,locale,license,*.git,objects,*.fr-fr.*,*.de-de.*,*.css,go.sum,*.key,gitignore,pyproject.toml,diff_test.go,go-licenses.json'
+# precise hits for CamelCased words,various other curious cases which require regex to ignore
+# entire line or some portion of it
+ignore-regex = '(\b(mx claus|commitT|ReadBy|#afile|respOne|commitI|[cC]rossReference)\b|shouldbe\.|women’s.*womens|"emoji":.*|,bu,|assert\.Equal.*"fo\b|github\.com/unknwon|Copyright 2014 Unknwon|allowed\.noone|[hH]eadErr|atLeast|{"\\U.*|\.metdata)'


if #26207 gets merged before this

Suggested change

ignore-regex = '(\b(mx claus|commitT|ReadBy|#afile|respOne|commitI|[cC]rossReference)\b|shouldbe\.|women’s.*womens|"emoji":.*|,bu,|assert\.Equal.*"fo\b|github\.com/unknwon|Copyright 2014 Unknwon|allowed\.noone|[hH]eadErr|atLeast|{"\\U.*|\.metdata)'

ignore-regex = '(\b(mx claus|commitT|ReadBy|#afile|respOne|commitI|[cC]rossReference)\b|shouldbe\.|women’s.*womens|"emoji":.*|,bu,|assert\.Equal.*"fo\b|github\.com/unknwon|Copyright 2014 Unknwon|allowed\.noone|[hH]eadErr|atLeast|{"\\U.*)'

#26194 (comment) There is no need to backport because these names are just used internal.

wolfogre

First of all, thank you so much for your contribution. However, I'm sorry if what I said next was rude.

IMO, if the typos were in comments, they wouldn't really hurt; if they were in code that could change logic, they have to be fixed one by one manually.

It could be quite easy for anyone to use a spell-checking tool and set up a workflow to check or correct typos for a repo, even if they know nothing about the repo. Then they would leave hundreds of changes for the maintainers to review, in order to ensure that nothing is broken. So what's the point? I don't see how it could possibly work and be merged into such a large repo. So it would just be a waste of the reviewer's time.

See also #26031.

Anyway, thanks again. I'm sure you're just trying to help.

silverwind · 2023-07-31T09:51:59Z

The approach with this running only on Actions is wrong. All our tools should be able to run locally without any dependency on a particular CI system, so it should instead add codespell to pyproject.toml via poetry add codespell and run it via poetry run codespell. Also I suggest configuring it via a pyproject.toml section as well.

yarikoptic · 2023-07-31T14:35:28Z

I will leave it to you guys to pick it up and complete the way you like since I hear different opinions. Cheers

yarikoptic added 2 commits July 27, 2023 19:48

Add github action to codespell main on push and PRs

3475a28

Add rudimentary codespell config

1b4db4c

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 28, 2023

pull-request-size Bot added the size/L label Jul 28, 2023

lunny added the topic/code-linting label Jul 28, 2023

lunny added this to the 1.21.0 milestone Jul 28, 2023

lunny previously approved these changes Jul 28, 2023

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jul 28, 2023

wxiaoguang reviewed Jul 28, 2023

View reviewed changes

wxiaoguang requested changes Jul 28, 2023

View reviewed changes

GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jul 28, 2023

wxiaoguang reviewed Jul 28, 2023

View reviewed changes

Comment thread docs/content/administration/config-cheat-sheet.en-us.md Outdated

KN4CK3R reviewed Jul 28, 2023

View reviewed changes

KN4CK3R mentioned this pull request Jul 28, 2023

Fix typo in metadata #26207

Merged

yarikoptic added 5 commits July 28, 2023 18:04

Add skips for codespell

d83508e

Do interactive fixing of leftover typos

3e12abc

ignore startd

1c6f306

ignore any .metdata -- to be fixed separately with migration (attn @K…

6a4e620

…N4CK3R)

[DATALAD RUNCMD] run codespell throughout

34cd783

=== Do not change lines below === { "chain": [], "cmd": "codespell -w", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^

yarikoptic force-pushed the enh-codespell branch from 539a98a to 34cd783 Compare July 28, 2023 22:11

yarikoptic commented Jul 28, 2023

View reviewed changes

silverwind pushed a commit that referenced this pull request Jul 30, 2023

Fix typo in metadata (#26207)

d08ab7b

#26194 (comment) There is no need to backport because these names are just used internal.

wolfogre requested changes Jul 31, 2023

View reviewed changes

yarikoptic closed this Jul 31, 2023

GiteaBot removed this from the 1.21.0 milestone Jul 31, 2023

go-gitea locked as resolved and limited conversation to collaborators Oct 30, 2023

	ignore-regex = '(\b(mx claus\|commitT\|ReadBy\|#afile\|respOne\|commitI\|[cC]rossReference)\b\|shouldbe\.\|women’s.womens\|"emoji":.\|,bu,\|assert\.Equal."fo\b\|github\.com/unknwon\|Copyright 2014 Unknwon\|allowed\.noone\|[hH]eadErr\|atLeast\|{"\\U.\|\.metdata)'
	ignore-regex = '(\b(mx claus\|commitT\|ReadBy\|#afile\|respOne\|commitI\|[cC]rossReference)\b\|shouldbe\.\|women’s.womens\|"emoji":.\|,bu,\|assert\.Equal."fo\b\|github\.com/unknwon\|Copyright 2014 Unknwon\|allowed\.noone\|[hH]eadErr\|atLeast\|{"\\U.)'

Uh oh!

Conversation

yarikoptic commented Jul 28, 2023

Uh oh!

wxiaoguang commented Jul 28, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wxiaoguang Jul 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wxiaoguang Jul 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wxiaoguang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yarikoptic commented Jul 28, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lunny commented Jul 28, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yarikoptic commented Jul 28, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wolfogre left a comment

Choose a reason for hiding this comment

Uh oh!

silverwind commented Jul 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yarikoptic commented Jul 31, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

wxiaoguang Jul 28, 2023 •

edited

Loading

wxiaoguang Jul 28, 2023 •

edited

Loading

silverwind commented Jul 31, 2023 •

edited

Loading